Add support of federated multisearch#571
Conversation
|
Hello @apt-TebbeM thank you for your PR! Let me know when the PR is ready 😊 |
…e the code more readable
|
I think it could be reviewed. I actually thought about chaning the API for the federated search part to something like this: public async Task<ISearchable<T>> FederatedMultiSearchAsync<T>(List<FederatedSearchQuery> queries,
MultiSearchFederationOptions federationOptions = default(MultiSearchFederationOptions),
CancellationToken cancellationToken = default)to make it less likely to have an empty list of queries. |
you can go ahead |
|
@curquiza when will this pr be reviewed? :) |
|
Hello @apt-TebbeM I'm sorry but I have a loooot on my plate right now. I will maybe be able to review your PR next week. I'll do my best. But busy work period at the moment But be sure I don't forget you! |
|
@apt-TebbeM if you are still here, can you answer @ahmednfwela point above 👆 so that we can improve or merge the PR? |
|
@curquiza @ahmednfwela I've updated the PR as suggested |
|
Is there still a desire for this to be merged? @ahmednfwela |
| /// <summary> | ||
| /// Always include property in json. MultiSearchFederationOptions will be serialized as "{}" | ||
| /// </summary> | ||
| public class MultiSearchFederationOptionsConverter : JsonConverter<MultiSearchFederationOptions> |
There was a problem hiding this comment.
I don't see what problem does this converter solve
There was a problem hiding this comment.
The API requiers a empty "federation" object when calling the /multisearch endpoint this is to respond in the expected format:
{
"hits": [ ...],
"processingTimeMs": 4,
"limit": 20,
"offset": 0,
"estimatedTotalHits": 11
}When the empty object parameter is not included it responds in a totaly different way:
{
"results": [
{
"indexUid": "searchIndex1",
"hits": [...],
"query": "w",
"processingTimeMs": 4,
"limit": 20,
"offset": 0,
"estimatedTotalHits": 11
},
{
"indexUid": "searchIndex2",
"hits": [...],
"query": "w",
"processingTimeMs": 0,
"limit": 20,
"offset": 0,
"estimatedTotalHits": 0
},
{
"indexUid": "searchIndex3",
"hits": [...],
"query": "w",
"processingTimeMs": 0,
"limit": 20,
"offset": 0,
"estimatedTotalHits": 0
}
]
}
|
What are the next steps here? @curquiza @ahmednfwela |
|
Is there still a chance to this into the package? |
curquiza
left a comment
curquiza
left a comment
There was a problem hiding this comment.
thank you and sorry for the delay
bors merge
curquiza
changed the title
|
Build succeeded: |
3 participants
Pull Request
Related issue
Fixes #560
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!